Skip to content
This repository was archived by the owner on Nov 22, 2018. It is now read-only.

We don't use the IMemoryCache from DI #172

Merged
merged 1 commit into from
Jul 27, 2018
Merged

We don't use the IMemoryCache from DI #172

merged 1 commit into from
Jul 27, 2018

Conversation

JunTaoLuo
Copy link
Contributor

@JunTaoLuo JunTaoLuo commented Jul 26, 2018 •
edited
Loading

@JunTaoLuo JunTaoLuo requested a review from Tratcher July 26, 2018 04:49
@Tratcher
Copy link
Member

Careful. Anybody that was resolving that memory cache can break if you remove it.

Is that why you're doing it only in Master?

@Eilon
Copy link
Member

Eilon commented Jul 26, 2018

Yeah this is a breaking change but I think a good one. I'd say it's fine for 3.0.

@JunTaoLuo
Copy link
Contributor Author

Right, it's a breaking change but we shouldn't be adding services we don't use.

@Eilon
Copy link
Member

Eilon commented Jul 26, 2018

I'm fine leaving 2.2 as-is, and fix only in 3.0. Mark the bug as breaking-change. (No need for an announcement unless someone complains.)

@Eilon
Copy link
Member

Eilon commented Jul 26, 2018

Actually, is there an issue for this? If not, please log one.

@JunTaoLuo
Copy link
Contributor Author

No there wasn't. Logged at: dotnet/aspnetcore#3360.

@JunTaoLuo JunTaoLuo merged commit ebec692 into master Jul 27, 2018
@JunTaoLuo JunTaoLuo deleted the johluo/no-memorycache-service branch July 27, 2018 23:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Tratcher Tratcher Tratcher approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JunTaoLuo @Tratcher @Eilon